Skip to content

subql changes make module reusable and minor fixes #361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Oct 31, 2024
Merged

subql changes make module reusable and minor fixes #361

merged 5 commits into from
Oct 31, 2024

Conversation

@DaMandal0rian
Copy link
Contributor

@DaMandal0rian DaMandal0rian commented Oct 31, 2024 •
edited
Loading

User description

  • remove gemini references in root module
  • make resources more generic
  • only create consensus subql instances and not nova
  • install nodejs on servers and improve installer script
  • fix hostname convention for instances
  • remove nginx as we will use caddy from astral repo

PR Type

enhancement

Description

  • Removed references to gemini and replaced them with more generic terms like subql.
  • Set instance counts to zero to prevent creation of instances.
  • Updated security group and VPC names to be more generic.
  • Changed default network name from gemini-3h to taurus.

Changes walkthrough 📝

Relevant files
Configuration changes
main.tf
Disable instance creation by setting count to zero             

explorer/terraform/aws/taurus/main.tf

  • Set instance-count-blue and instance-count-green to 0.
  • Commented out previous instance count variables.
    		•  +2/-2     
    instances.tf
    Update security group references for instances                     

    templates/terraform/subql/base/instances.tf

  • Updated security group references from gemini-subql-sg to subql-sg.
    		•  +4/-4     
    network.tf
    Rename VPC and security group for generic use                       

    templates/terraform/subql/base/network.tf

  • Renamed VPC and security group from gemini-subql to subql.
  • Updated VPC ID references in subnet, internet gateway, and route table
    resources.
    		•  +7/-7     
    outputs.tf
    Update output variable for new security group name             

    templates/terraform/subql/base/outputs.tf

  • Updated output variable to reflect new security group name subql-sg.
    		•  +1/-1     
    variables.tf
    Update default network name to `taurus`                                   

    templates/terraform/subql/base/variables.tf

    • Changed default network name from gemini-3h to taurus.
    		 +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @github-actions
    Copy link

    github-actions bot commented Oct 31, 2024

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Instance Count
    The instance count for both blue and green configurations is hardcoded to zero, which might not be intended for production or staging environments. This could potentially halt the deployment of any instances.

    Security Group Reference
    The security group IDs have been updated to a new security group without verifying if the new group has the correct configurations and rules as the previous one. This could lead to potential security issues if the new group is not configured properly.

    VPC Configuration
    The VPC and related networking resources have been renamed and reconfigured. It's crucial to ensure that these changes do not disrupt existing infrastructure or lead to misconfigurations that could affect network security and connectivity.

    @github-actions
    Copy link

    github-actions bot commented Oct 31, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Replace hardcoded instance counts with variable values to enable dynamic scaling

    Ensure that the instance-count-blue and instance-count-green are set to use the
    variable values instead of hardcoded zeros to maintain dynamic scaling capabilities.

    explorer/terraform/aws/taurus/main.tf [43-57]

    -instance-count-blue = 0 #var.instance_count_blue
-instance-count-green = 0 #var.instance_count_green
+instance-count-blue = var.instance_count_blue
+instance-count-green = var.instance_count_green
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a critical issue by replacing hardcoded instance counts with variable values, which is essential for maintaining dynamic scaling capabilities. It directly impacts the functionality and flexibility of the infrastructure configuration.

    		9
    Possible issue
    Ensure the security group ID correctly corresponds to the intended settings for each instance

    Verify the security group ID reference to ensure it matches the intended security
    group for each instance type, as the change from gemini-subql-sg to subql-sg might
    not reflect the correct security group settings.

    templates/terraform/subql/base/instances.tf [8-210]

    -vpc_security_group_ids = ["${aws_security_group.subql-sg.id}"]
+vpc_security_group_ids = ["${aws_security_group.correct-sg.id}"]  # Replace 'correct-sg' with the actual intended security group
    Suggestion importance[1-10]: 7

    Why: The suggestion prompts verification of security group IDs, which is important for ensuring that instances have the correct security settings. However, it is not directly actionable as it requires user verification and does not provide a specific fix.

    		7
    Validate that VPC configurations are correct and align with network requirements

    Confirm that the new VPC and subnet configurations align with the network
    architecture requirements, especially changes in VPC IDs which could impact
    connectivity and security settings.

    templates/terraform/subql/base/network.tf [15-42]

    -vpc_id = aws_vpc.subql-vpc.id
+vpc_id = aws_vpc.correct-vpc.id  # Replace 'correct-vpc' with the actual intended VPC
    Suggestion importance[1-10]: 7

    Why: This suggestion highlights the need to confirm VPC configurations, which is crucial for maintaining proper network architecture and security. Like the previous suggestion, it requires user verification and does not offer a specific solution.

    		7
    Enhancement
    Update output values to reflect the correct security group rules

    Update the output for ingress_rules to ensure it reflects the correct security group
    rules following the security group ID updates.

    templates/terraform/subql/base/outputs.tf [4]

    -value = aws_security_group.subql-sg.*.ingress
+value = aws_security_group.updated-sg.*.ingress  # Replace 'updated-sg' with the actual intended security group
    Suggestion importance[1-10]: 6

    Why: The suggestion to update output values ensures that the correct security group rules are reflected, which is important for accurate monitoring and management of security settings. However, it is not directly actionable and requires user intervention to determine the correct security group.

    		6

    @DaMandal0rian
    subql changes make module reusable
    373d7f3 
    - remove gemini references
- make resources more generic
- install nodejs
    @DaMandal0rian DaMandal0rian changed the title subql changes make module reusable subql changes make module reusable and minor fixes Oct 31, 2024
    @DaMandal0rian DaMandal0rian merged commit 5ace78a into main Oct 31, 2024
    1 check passed
    @DaMandal0rian DaMandal0rian deleted the feat/subql branch October 31, 2024 19:22
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @marc-aurele-besner marc-aurele-besner Awaiting requested review from marc-aurele-besner

    Assignees

    No one assigned

    Labels

    Review effort [1-5]: 3

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    3 participants

    @DaMandal0rian @marc-aurele-besner